Skip to content

REFACTOR: Promote token-smuggling & misc converter defaults to ClassVar - #1967

Closed
Roman Lutz (romanlutz) wants to merge 1 commit into
microsoft:mainfrom
romanlutz:romanlutz/refactor-token-smuggling-classvar
Closed

REFACTOR: Promote token-smuggling & misc converter defaults to ClassVar#1967
Roman Lutz (romanlutz) wants to merge 1 commit into
microsoft:mainfrom
romanlutz:romanlutz/refactor-token-smuggling-classvar

Conversation

@romanlutz

Copy link
Copy Markdown
Contributor

REFACTOR: Promote token-smuggling & misc converter defaults to ClassVar

Part of the ongoing class-constants audit (see #1964, #1965). Pure structural rename — no value or behaviour changes.

What changed

Promotes hard-coded __init__ defaults onto the owning class as ClassVar constants, using the sentinel-default pattern documented in the audit kickoff so subclasses can inherit the default without restating it.

pyrit/prompt_converter/token_smuggling/

The four-class SmugglerConverter family all shared action: Literal["encode", "decode"] = "encode". The default now lives once on the base:

class SmugglerConverter(PromptConverter, abc.ABC):
    DEFAULT_ACTION: ClassVar[Literal["encode", "decode"]] = "encode"

    def __init__(self, action: Literal["encode", "decode"] | None = None) -> None:
        if action is None:
            action = self.DEFAULT_ACTION
        ...

The three subclasses (AsciiSmugglerConverter, SneakyBitsSmugglerConverter, VariationSelectorSmugglerConverter) now accept action: Literal["encode", "decode"] | None = None and delegate resolution to the base via super().__init__(action=action). Their existing _brick_legacy_init = True opt-out (positional-arg deprecation grandfathering, scheduled for removal in 0.16.0) is preserved unchanged.

pyrit/prompt_converter/unicode_sub_converter.py

UnicodeSubstitutionConverter had a lone hard-coded start_value: int = 0xE0000. Promoted to DEFAULT_START_VALUE: ClassVar[int] = 0xE0000 and the constructor uses the same sentinel pattern.

Conventions followed

  • ClassVar[T] annotation on every promoted constant.
  • PEP 604 unions (X | None) for the sentinel parameter type.
  • Constants placed near the top of the class body (per the style-guide method-ordering checklist).
  • Docstrings updated to reference the DEFAULT_* constant by name.
  • _brick_legacy_init and positional signatures preserved for the smuggler family (no signature changes).

Verification

uv run --link-mode=copy ruff check pyrit/prompt_converter/token_smuggling/ pyrit/prompt_converter/unicode_sub_converter.py
uv run --link-mode=copy ty check pyrit/prompt_converter/token_smuggling/ pyrit/prompt_converter/unicode_sub_converter.py
uv run --link-mode=copy pytest tests/unit/prompt_converter/ -x --no-header -q

All green — 1012 passed, 38 skipped (no test changes needed; the public default is unchanged).

Hoist the shared ``action='encode'`` default for the SmugglerConverter family
onto ``SmugglerConverter.DEFAULT_ACTION: ClassVar[Literal['encode', 'decode']]``
and switch the three subclasses (AsciiSmugglerConverter, SneakyBitsSmugglerConverter,
VariationSelectorSmugglerConverter) to the sentinel-default pattern so they inherit
the base's default without restating the literal. Also promote
UnicodeSubstitutionConverter's ``start_value=0xE0000`` to
``DEFAULT_START_VALUE: ClassVar[int]`` via the same sentinel pattern.

No value changes. No behaviour changes.

Verified: ruff / ty / pytest tests/unit/prompt_converter/.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@romanlutz

Copy link
Copy Markdown
Contributor Author

Closing per maintainer feedback on the constants-audit work.

This PR introduced the sentinel-default pattern (changing x: T = <literal> into x: T | None = None + if x is None: x = self.DEFAULT_X) which widens the public API signature. The preference is to keep constants inside their owning class (Track A — see #1964, #1965, #1972, #1973, #1974, #1975) but not to promote hard-coded single-use defaults via a None-sentinel.

Branch left intact in case any portion is worth cherry-picking later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants